-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(node): add and modify node.Status.Conditions #3065
Conversation
9bf320b
to
8d84685
Compare
8d84685
to
84a4de8
Compare
84a4de8
to
e79424a
Compare
e79424a
to
8166a5c
Compare
8166a5c
to
92555f3
Compare
92555f3
to
3a9ddaa
Compare
3a9ddaa
to
ff0a01d
Compare
b207776
to
13a2fd7
Compare
79236cb
to
8b1028e
Compare
8b1028e
to
5f9e000
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, LGTM, but need to fix the parse function for the the config file and add a unit test cases for the newly introduced functions.
5f9e000
to
4687cac
Compare
9b5f351
to
e54b87e
Compare
Check if the module `dm_crypt` is enabled as the first module. Add a unit test to check the kernel modules condition. ref: longhorn/longhorn 9153 Signed-off-by: James Lu <james.lu@suse.com>
e54b87e
to
844d20c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the effort.
@Mergifyio backport v1.7.x |
✅ Backports have been created
|
Which issue(s) this PR fixes:
Issue # longhorn/longhorn#9153
What this PR does / why we need it:
Add the node.Status.Condition
ModulesLoaded
and check if the moduledm_crypt
is enabled.Special notes for your reviewer:
Additional documentation or context